Fix documentation, requirements, and subsystem test suite issues identified in formal review by Copilot · Pull Request #44 · demaconsulting/ReviewMark

Copilot · 2026-04-03T22:31:32Z

Formal reviews were performed on all 14 review-sets defined in .reviewmark.yaml. The following issues were identified and fixed.

Documentation fixes (`docs/design/`)

File	Change
`cli/context.md`	Correct `DefinitionFile` type from `string` to `string?`
`configuration/review-mark-configuration.md`	Add `none` evidence-source type documentation; clarify fingerprint algorithm converts hashes to lowercase hex strings before sorting
`indexing/review-index.md`	Replace undocumented `GetStatus()` section with documentation for the three actual query methods (`GetEvidence`, `HasId`, `GetAllForId`) and both `Save()` overloads
`self-test/validation.md`	Correct exit-code mechanism: calls `Context.WriteError()` which causes `ExitCode` to return non-zero (rather than directly assigning `ExitCode`)
`indexing/path-helpers.md`	Normalise title to `# PathHelpers` (consistent with all other unit design documents)

Requirements fixes (`docs/reqstream/`)

File	Change
`configuration/review-mark-configuration.yaml`	Remove duplicate test name `ReviewMarkConfiguration_Load_NonExistentFile_ReturnsNullConfigWithErrorIssue`
`configuration/glob-matcher.yaml`	Add 2 undocumented tests (`GlobMatcher_GetMatchingFiles_EmptyPatterns_ReturnsEmptyList`, `GlobMatcher_GetMatchingFiles_MultipleIncludePatterns_ReturnsAllMatching`)
`indexing/path-helpers.yaml`	Add 3 undocumented tests (`PathHelpers_SafePathCombine_DoubleDotsInMiddle_ThrowsArgumentException`, `PathHelpers_SafePathCombine_CurrentDirectoryReference_CombinesCorrectly`, `PathHelpers_SafePathCombine_EmptyRelativePath_ReturnsBasePath`)
`indexing/indexing.yaml`	Add 5 missing error-handling tests to `ReviewMark-Indexing-ScanPdfEvidence`

Subsystem test suites (new)

Four new subsystem integration test suite files have been added — one per subsystem — so that subsystem review-sets contain subsystem-level evidence rather than unit-level test files:

File	Tests
`test/.../Cli/CliTests.cs`	`Cli_VersionFlag_OutputsVersionOnly`, `Cli_HelpFlag_OutputsUsageInformation`, `Cli_ValidateFlag_RunsValidation`, `Cli_SilentFlag_SuppressesOutput`
`test/.../Configuration/ConfigurationTests.cs`	`Configuration_LoadConfig_ResolvesNeedsReviewFiles`, `Configuration_LoadConfig_FingerprintReflectsFileContent`, `Configuration_LoadConfig_PlanGenerationSucceeds`
`test/.../Indexing/IndexingTests.cs`	`Indexing_SafePathCombine_WithIndexPath_LoadsIndex`, `Indexing_ReviewIndex_SaveAndLoad_RoundTrip`
`test/.../SelfTest/SelfTestTests.cs`	`SelfTest_Run_AllTestsPass_ExitCodeIsZero`, `SelfTest_Run_GeneratesResultsFile`

Subsystem requirements — `children` links and `.reviewmark.yaml` review-sets

All subsystem requirements (cli.yaml, configuration.yaml, indexing.yaml, self-test.yaml) have been updated with children links pointing down to the relevant unit requirement IDs, enabling transitive test coverage traceability.

The four subsystem review-sets in .reviewmark.yaml have been updated to follow the 3-artifact pattern — subsystem-requirements + subsystem-design + subsystem-test-suite — replacing the previous mix of unit design files and unit test files.

Pull Request

Description

Addresses documentation, requirements, and structural issues identified during formal reviews of all 14 review-sets in .reviewmark.yaml. Changes cover design documentation corrections, requirements traceability improvements, new subsystem test suites, and subsystem requirements hierarchy (children links). A markdown line-length lint violation introduced in review-mark-configuration.md has also been corrected.

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Code quality improvement

Related Issues

Pre-Submission Checklist

Before submitting this pull request, ensure you have completed the following:

Build and Test

Code builds successfully: dotnet build --configuration Release
All unit tests pass: dotnet test --configuration Release
Self-validation tests pass:
dotnet run --project src/DemaConsulting.ReviewMark --configuration Release --framework net10.0
--no-build -- --validate
Code produces zero warnings

Code Quality

Code formatting is correct: dotnet format --verify-no-changes
New code has appropriate XML documentation comments
Static analyzer warnings have been addressed

Quality Checks

Please run the following checks before submitting:

Spell checker passes: cspell "**/*.{md,cs}"
Markdown linter passes: markdownlint "**/*.md"
YAML linter passes: yamllint .

Testing

Added unit tests for new functionality
Updated existing tests if behavior changed
All tests follow the AAA (Arrange, Act, Assert) pattern
Test coverage is maintained or improved

Documentation

Updated README.md (if applicable)
Updated ARCHITECTURE.md (if applicable)
Added code examples for new features (if applicable)
Updated requirements.yaml (if applicable)

Additional Notes

All 175 tests pass across .NET 8, 9, and 10. Build is clean (0 warnings). All lint checks pass (yamllint, cspell, markdownlint-cli2, dotnet format). dotnet reqstream --lint reports no issues.

…iles Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>

Malcolmnixon · 2026-04-03T22:36:04Z

@copilot There don't appear to be subsystem test suites. Subsystems should have their own test suites and the requirements are supposed to link to them.

Subsystem reviews must consist of the subsystem-requirements, subsystem-design, and subsystem-test-suite.

Unit reviews must consist of the unit-requirements, unit-design, unit-code, and unit-test-suite.

Subsystem requirements should 'children' requirement-link to the relevant unit requirements.

- Add CliTests.cs: subsystem integration tests for CLI (Context + Program) - Add ConfigurationTests.cs: subsystem integration tests for Configuration - Add IndexingTests.cs: subsystem integration tests for Indexing - Add SelfTestTests.cs: subsystem integration tests for SelfTest - Update cli.yaml: add children links and new subsystem test references - Update configuration.yaml: add children links and new subsystem test references - Update indexing.yaml: add children links and new subsystem test references - Update self-test.yaml: add children links and new subsystem test references - Update .reviewmark.yaml: update subsystem review-sets to include new test files Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>

Copilot · 2026-04-03T23:17:17Z

@copilot There don't appear to be subsystem test suites. Subsystems should have their own test suites and the requirements are supposed to link to them.

Subsystem reviews must consist of the subsys...

Done in commit d3e5f08. The following changes address both points:

Subsystem test suites created (one per subsystem):

test/.../Cli/CliTests.cs — integration tests for Context + Program pipeline (version, help, validate, silent flags)
test/.../Configuration/ConfigurationTests.cs — integration tests for ReviewMarkConfiguration + GlobMatcher (needs-review resolution, fingerprint, plan generation)
test/.../Indexing/IndexingTests.cs — integration tests for ReviewIndex + PathHelpers (safe-path-combine+load, save/load round-trip)
test/.../SelfTest/SelfTestTests.cs — integration tests for the SelfTest subsystem (all-pass, results file)

children requirement links added to every subsystem requirement in cli.yaml, configuration.yaml, indexing.yaml, and self-test.yaml, pointing down to the relevant unit requirement IDs.

.reviewmark.yaml subsystem review-sets updated to the 3-artifact pattern: subsystem requirements + subsystem design + subsystem test suite (unit design files and unit test files removed from subsystem review-sets).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/1108766/chrome-linux.zip
- Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js (http block)
- Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js ./.github/workfl--noprofile /usr/lib/update-notifier/package-data-downloader ./re�� ./.versionmark.yaml ./.markdownlint-cli2.yaml git ./docs/requiremebash ./docs/reqstream--norc ./docs/reqstream--noprofile bash --no�� maConsulting.ReviewMark.Tests/SelfTest/SelfTestTests.cs ./docs/reqstream/review-mark/platform-requirements.yaml cal/bin/bash (http block)
- Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js ep/bin/linux-x64--noprofile bash k/Re�� --noprofile ep/bin/linux-x64/rg p/bin/git (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Malcolmnixon · 2026-04-03T23:21:56Z

@copilot Please fix the issues reported by running the lint scripts

Copilot

Pull request overview

This pull request aligns ReviewMark’s formal review artifacts (design docs, ReqStream requirements, and evidence/test suites) with the actual implementation and test coverage, and updates subsystem review-sets to follow the intended 3-artifact pattern (requirements + design + subsystem test suite).

Changes:

Correct and extend design documentation for evidence sources, fingerprinting, ReviewIndex APIs, and self-test exit code behavior.
Improve requirements traceability by adding missing test mappings and introducing children links for subsystem-to-unit hierarchy.
Add subsystem-level integration test suites for CLI, Configuration, Indexing, and SelfTest; update .reviewmark.yaml subsystem review-sets accordingly.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
test/DemaConsulting.ReviewMark.Tests/Cli/CliTests.cs	Adds CLI subsystem integration tests for version/help/validate/silent behaviors.
test/DemaConsulting.ReviewMark.Tests/Configuration/ConfigurationTests.cs	Adds Configuration subsystem integration tests spanning config loading, fingerprinting, and plan generation.
test/DemaConsulting.ReviewMark.Tests/Indexing/IndexingTests.cs	Adds Indexing subsystem integration tests combining PathHelpers + ReviewIndex load/save behaviors.
test/DemaConsulting.ReviewMark.Tests/SelfTest/SelfTestTests.cs	Adds SelfTest subsystem integration tests for validation exit code and results file generation.
docs/reqstream/review-mark/cli/cli.yaml	Adds `children` links and maps new CLI subsystem tests into requirements.
docs/reqstream/review-mark/configuration/configuration.yaml	Adds `children` links and maps new Configuration subsystem tests into requirements.
docs/reqstream/review-mark/configuration/glob-matcher.yaml	Documents additional GlobMatcher unit tests previously missing from requirements.
docs/reqstream/review-mark/configuration/review-mark-configuration.yaml	Removes duplicate test mapping entry.
docs/reqstream/review-mark/indexing/indexing.yaml	Adds missing test mappings and `children` links for subsystem-to-unit traceability.
docs/reqstream/review-mark/indexing/path-helpers.yaml	Documents additional PathHelpers unit tests previously missing from requirements.
docs/reqstream/review-mark/self-test/self-test.yaml	Adds subsystem test mappings and `children` links pointing to Validation unit requirements.
docs/design/review-mark/cli/context.md	Corrects `DefinitionFile` type documentation to `string?`.
docs/design/review-mark/configuration/review-mark-configuration.md	Documents evidence-source `none` and clarifies fingerprinting hash normalization/sorting.
docs/design/review-mark/indexing/path-helpers.md	Normalizes document title to match unit doc naming convention.
docs/design/review-mark/indexing/review-index.md	Replaces outdated `GetStatus()` docs with the actual query/persistence APIs.
docs/design/review-mark/self-test/validation.md	Corrects description of non-zero exit code mechanism via `Context.WriteError()`.
.reviewmark.yaml	Updates subsystem review-sets to include subsystem requirements + subsystem design + subsystem test suite.
.cspell.yaml	Adds `selftest` to allowlist for spell checking.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Agent-Logs-Url: https://github.com/demaconsulting/ReviewMark/sessions/4e67c1ba-0bd1-4537-b878-2b85e42f4d22 Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>

Copilot · 2026-04-03T23:26:40Z

@copilot Please fix the issues reported by running the lint scripts

Fixed in commit c092dab. The only lint failure was an MD013 line-length violation in docs/design/review-mark/configuration/review-mark-configuration.md (line 26 was 131 chars, limit is 120). The none evidence-source description in the table was shortened. All 4 lint checks now pass: yamllint, cspell, markdownlint-cli2, dotnet format.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/1108766/chrome-linux.zip
- Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node install.js (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

fix: address formal review issues in documentation and requirements f…

10b8797

…iles Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>

Copilot AI assigned Copilot and Malcolmnixon Apr 3, 2026

Copilot created this pull request from a session on behalf of Malcolmnixon April 3, 2026 22:32 View session

Copilot finished work on behalf of Malcolmnixon April 3, 2026 22:32

Copilot AI requested a review from Malcolmnixon April 3, 2026 22:32

Copilot started work on behalf of Malcolmnixon April 3, 2026 22:36 View session

Copilot AI and others added 2 commits April 3, 2026 23:01

fix: add 'selftest' to cspell word list

bc8f4b9

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>

Copilot AI changed the title ~~Fix documentation and requirements issues identified in formal review~~ Fix documentation, requirements, and subsystem test suite issues identified in formal review Apr 3, 2026

Copilot finished work on behalf of Malcolmnixon April 3, 2026 23:18

Malcolmnixon marked this pull request as ready for review April 3, 2026 23:19

Copilot AI review requested due to automatic review settings April 3, 2026 23:19

Copilot started reviewing on behalf of Malcolmnixon April 3, 2026 23:20 View session

Copilot started work on behalf of Malcolmnixon April 3, 2026 23:22 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

Copilot finished work on behalf of Malcolmnixon April 3, 2026 23:27

Malcolmnixon approved these changes Apr 3, 2026

View reviewed changes

Malcolmnixon merged commit f658cc5 into main Apr 3, 2026
15 checks passed

Malcolmnixon deleted the copilot/fix-review-sets-issues branch April 3, 2026 23:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix documentation, requirements, and subsystem test suite issues identified in formal review#44

Fix documentation, requirements, and subsystem test suite issues identified in formal review#44
Malcolmnixon merged 4 commits intomainfrom
copilot/fix-review-sets-issues

Copilot AI commented Apr 3, 2026 •

edited

Loading

Uh oh!

Malcolmnixon commented Apr 3, 2026

Uh oh!

Copilot AI commented Apr 3, 2026 •

edited

Loading

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Malcolmnixon commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI commented Apr 3, 2026 •

edited

Loading

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation fixes (docs/design/)

Requirements fixes (docs/reqstream/)

Subsystem test suites (new)

Subsystem requirements — children links and .reviewmark.yaml review-sets

Pull Request

Description

Type of Change

Related Issues

Pre-Submission Checklist

Build and Test

Code Quality

Quality Checks

Testing

Documentation

Additional Notes

Uh oh!

Malcolmnixon commented Apr 3, 2026

Uh oh!

Copilot AI commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Malcolmnixon commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Apr 3, 2026 •

edited

Loading

Documentation fixes (`docs/design/`)

Requirements fixes (`docs/reqstream/`)

Subsystem requirements — `children` links and `.reviewmark.yaml` review-sets

Copilot AI commented Apr 3, 2026 •

edited

Loading

Copilot AI commented Apr 3, 2026 •

edited

Loading